Skip to content

Comments

feat: ServiceAccount with ID token option#4474

Open
lpezet wants to merge 15 commits intogoogle:mainfrom
lpezet:feat/service-account-id-token
Open

feat: ServiceAccount with ID token option#4474
lpezet wants to merge 15 commits intogoogle:mainfrom
lpezet:feat/service-account-id-token

Conversation

@lpezet
Copy link

@lpezet lpezet commented Feb 13, 2026

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
ServiceAccountCredentialExchanger implementation is restricted to OAuth2-type tokens (access token).
For Service-to-Service authentication, ID token+audience is needed.

Solution:
Provide attributes in ServiceAccount auth config to specify ID token-type auth.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

NB: Some of test_cli_deploy.py fail for me but I have not changed anything there:

FAILED tests/unittests/cli/utils/test_cli_deploy.py::TestValidateAgentImport::test_success_with_app_export - click.exceptions.ClickException: Failed to import agent module:
FAILED tests/unittests/cli/utils/test_cli_deploy.py::TestValidateAgentImport::test_raises_on_import_error - assert 'nonexistent_module' in "Failed to import agent module:\nNo module named 'test_raises_on_import_error0'\n\nPlease ensure all dependencies are ... omitting --validate-agent-import (default) or passing --skip-agent-import-validat...
FAILED tests/unittests/cli/utils/test_cli_deploy.py::TestValidateAgentImport::test_cleans_up_sys_modules - click.exceptions.ClickException: Failed to import agent module:

Manual End-to-End (E2E) Tests:

In my setup I have a custom MCP Server deployed to Cloud Run running under a SA "producer", NOT public, and with a SA "consumer" granted "role/run.invoker" on that service.

  1. Create VM instance with SA "consumer"
gcloud compute instances create adk-test-instance \
    --zone=us-central1-a \
    --machine-type=e2-micro \
    --image-family=ubuntu-2204-lts \
    --image-project=ubuntu-os-cloud \
    --service-account=$SA \
    --scopes=https://www.googleapis.com/auth/cloud-platform
  1. SSH into instance
gcloud compute ssh adk-test-instance --zone=$ZONE
  1. Clone branch and set things up
git clone --single-branch --branch feat/service-account-id-token https://github.com/lpezet/adk-python.git
cd adk-python
curl -LsSf https://astral.sh/uv/install.sh | sh
source $HOME/.local/bin/env
uv venv --python "python3.11" ".venv"
source .venv/bin/activate
uv sync --all-extras
  1. Create test
mkdir tests/integration/fixture/adk_id_token/
cd tests/integration/fixture/adk_id_token/
echo "from . import agent" > __init__.py
vi agent.py

Paste the following:

import os
from google.adk import Agent
from google.genai import types

from google.adk.tools.mcp_tool.mcp_toolset import McpToolset
from google.adk.tools.mcp_tool.mcp_session_manager import StreamableHTTPConnectionParams
from google.adk.auth.auth_credential import AuthCredential, AuthCredentialTypes, ServiceAccount

from fastapi.openapi.models import OAuth2
from fastapi.openapi.models import OAuthFlowClientCredentials
from fastapi.openapi.models import OAuthFlows

# Check necessary environment variables
MCP_BASE_URL = os.getenv("MCP_BASE_URL") # custom remote MCP server base URL, deployed in say Cloud Run or GCE, e.g. "https://your-service-xyz-uc.a.run.app"
if not MCP_BASE_URL:
  raise ValueError(
      "MCP_BASE_URL environment variable is not set. Please set it"
      " to the MCP URL."
  )

SCOPES = {"https://www.googleapis.com/auth/cloud-platform": ""}

auth_scheme=OAuth2(
    flows=OAuthFlows(
        clientCredentials=OAuthFlowClientCredentials(
            tokenUrl="https://oauth2.googleapis.com/token",
            scopes=SCOPES,
        )
    )
)
auth_credential = AuthCredential(
    auth_type=AuthCredentialTypes.SERVICE_ACCOUNT, service_account=ServiceAccount(use_default_credential = True, scopes=SCOPES.keys(), token_kind="id_token", audience=MCP_BASE_URL)
)
mcp_toolset = McpToolset(
    connection_params=StreamableHTTPConnectionParams(url=f"{MCP_BASE_URL}/mcp"),
    auth_scheme=auth_scheme,
    auth_credential=auth_credential,
)

root_agent = Agent(
    model="gemini-2.5-flash",
    name="agent",
    description="A helpful assistant to talk to.",
    instruction="""You are a helpful assistant to talk to.
    """,
    tools=[mcp_toolset],
)
  1. Run adk
cd tests/integration/fixture/
ask run adk_id_token
  1. Test MCP tool calls

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@lpezet lpezet changed the title ServiceAccount with ID token option feat: ServiceAccount with ID token option Feb 13, 2026
@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label Feb 13, 2026
@ryanaiagent ryanaiagent self-assigned this Feb 13, 2026
@ryanaiagent
Copy link
Collaborator

/gemini review

@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@ryanaiagent
Copy link
Collaborator

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds the ability to use ID tokens with Service Accounts, which is a great feature for service-to-service authentication. The implementation looks solid, with changes to the ServiceAccount model, the credential exchanger logic, and corresponding unit tests.

I've left a few comments on potential improvements:

  • In service_account_exchanger.py, there are a couple of redundant checks for service_account_credential that can be removed to simplify the code, as this is already handled at the beginning of the function. I also noticed an opportunity to reuse a Request object for better performance and consistency.
  • In the new tests, there's a redundant monkeypatch call that can be removed.

Overall, the changes are well-implemented and tested. Addressing these minor points will improve code clarity and maintainability.

@ryanaiagent
Copy link
Collaborator

Hi @lpezet , Thank you for your contribution! We appreciate you taking the time to submit this pull request.

  1. Can you implement the bot suggestions.
  2. Fix the failing unit tests and mypydiff tests.
  3. Also run autoformat.sh to fix the formatting errors.

@ryanaiagent ryanaiagent added the request clarification [Status] The maintainer need clarification or more information from the author label Feb 19, 2026
@lpezet
Copy link
Author

lpezet commented Feb 20, 2026

@ryanaiagent

  1. I implemented the bot suggestions.
  2. I have no idea how to fix the dependencies error mentioned in most of the failed checks (which now disappeared after pushing my changes and forgot to copy the errors).
  3. I ran autoformat.sh, pytest ./tests/unittests, uv build, and tested "the locally built wheel file" (as per CONTRIBUTING.md.
    The only files affected by autoformat.sh are files I did not touch:
$ ./autoformat.sh 
---------------------------------------
|  Organizing imports for src/...
---------------------------------------
All done! ✨ 🍰 ✨
---------------------------------------
|  Organizing imports for tests/...
---------------------------------------
All done! ✨ 🍰 ✨
---------------------------------------
|  Organizing imports for contributing/...
---------------------------------------
All done! ✨ 🍰 ✨
---------------------------------------
|  Auto-formatting src/...
---------------------------------------
All done! ✨ 🍰 ✨
452 files left unchanged.
---------------------------------------
|  Auto-formatting tests/...
---------------------------------------
All done! ✨ 🍰 ✨
388 files left unchanged.
---------------------------------------
|  Auto-formatting contributing/...
---------------------------------------
All done! ✨ 🍰 ✨
321 files left unchanged.
$ gs
On branch feat/service-account-id-token
Your branch is up to date with 'origin/feat/service-account-id-token'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   contributing/samples/gepa/experiment.py
        modified:   contributing/samples/gepa/run_experiment.py

no changes added to commit (use "git add" and/or "git commit -a")

I've been burnt before contributing to other Google projects, and this really is an Auth concern that, as other comments in the code mentioned, might need more thoughts than my little PR.

READ: I'm not going to do much more unless you can tell me this has high chances to be merged.

@ryanaiagent
Copy link
Collaborator

ryanaiagent commented Feb 20, 2026

Hi @lpezet , I appreciate the contribution and I completely understand the hesitation—I want to make sure your effort here is well-spent and doesn’t result in a 'burnt' experience.

Regarding the Auth concerns you noted: you’re right to call those out. They are exactly why a deep architectural review is necessary. However, our process is to perform that deep dive only once the PR is in a 'Green' state and adhering to our contributing guide.

If you can get the CI passing, I’ll prioritize a review of the logic to give you a definitive answer on the merge path. I want to ensure we're aligned on the design before we go any further.

As it stands your PR is still failing mypy-diff test. You can ignore the mypy test.

@lpezet
Copy link
Author

lpezet commented Feb 20, 2026

I remember now: I added the extra value check before exactly because of this error:

src/google/adk/tools/openapi_tool/auth/credential_exchangers/service_account_exchanger.py:: error: Item "None" of "ServiceAccountCredential | None" has no attribute "model_dump"  [union-attr]

Somehow, even with the previous "if" logic raising AuthCredentialMissingError when auth_credential.service_account.service_account_credential is None and not auth_credential.service_account.use_default_credential is true, mypy would not catch that.
But adding that extra check made Gemini complain that it was redundant.

It's a mypy narrowing limitation I believe. So either I add back that "redundant" code (which I just did, for now, with a comment to explain the redundancy), or...? What do you want me to do here?

That should fix the following error as well (using uv run mypy . --strict and comparing new errors vs. existing errors).
I added that step on my end too, to check for that (something that's missing from CONTRIBUTING.md by the way).

@ryanaiagent
Copy link
Collaborator

ryanaiagent commented Feb 20, 2026

Hi @lpezet , you need to fix the failing unit test.
The CI test test_service_account_id_token_requires_audience is failing because the ServiceAccount model was missing the validation to actually enforce the audience requirement.
You had this validation earlier. But you removed it to fix mypy diff error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core [Component] This issue is related to the core interface and implementation request clarification [Status] The maintainer need clarification or more information from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ServiceAccountCredentialExchanger: ID Token kind

3 participants